-
-
Notifications
You must be signed in to change notification settings - Fork 654
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[ADD] product_customer_code_picking module v.7.0 #2
[ADD] product_customer_code_picking module v.7.0 #2
Conversation
res[move.id] = '' | ||
partner = move.picking_id.partner_id | ||
if move.picking_id.partner_id.parent_id: | ||
partner = move.picking_id.partner_id.parent_id |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi,
This section of code seems a little restrictive. Consider a 2 level deep partner. It would only take the direct parent. I could be wrong but it feels like what you might want here is a list of ids and use a ('partner_id', in, partner_ids) in your search.
Just a general comment, maybe there is a good reason for ignoring the current partner and any higher up partners.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agree. Also see my comment at https://code.launchpad.net/~agilebg/stock-logistic-flows/adding_product_customer_code_picking/+merge/202472/comments/537661
I'm about to apply some fix to this PR
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some improvements about this at tafaRU#3
The 7.0 branch is now green, please rebase on 7.0 so that we can see if this branch is green as well. Thanks! |
On 09/19/2014 03:57 PM, Leonardo Pistone wrote:
Uhm, the branch is old and doesn't contain .travis.yml |
Needs a rebase in order to be validated by Travis. @tafaRU could you do this, or should someone copy this branch? |
Rebasing I get a conflict on the file product_customer_code_picking/stock_picking_view.xml, a bit strange seeing that this branch adds a new module. So I prefer merging instead rebasing. |
I think now the PR can be merged! |
"description": """ | ||
Based on product_customer_code, this module loads in every stock picking | ||
the customer code defined in the product. | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggested description improvement:
"This module makes the product customer code visible in the stock moves of a picking. "
@tafaRU if you're ok with it, could you apply the suggest description improvement above. I'll then merge the PR. |
@gurneyalex just modified the description. Thanks. |
It looks like all remarks have been addressed. Thanks to all! |
[ADD] product_customer_code_picking module v.7.0
Use @api.multi for has_valuation_move() + fix log message
…eparation_line_lb remove mail thread dep
stock_auto_move : replace utf8 by utf-8, use camelcase in class name
mprove import/export of group_ids and user_ids
[10.0] [FIX] [stock_no_negative] proper indentation, docstring, flake8
Actualizacion rama 11
My feature branch
Moving the MP: https://code.launchpad.net/~agilebg/stock-logistic-flows/adding_product_customer_code_picking/+merge/202472